More progress on broadcast feature #944
Conversation
|
It's minor but not sure tournament is the right word to switch between open and gm specific. |
|
Very impressive work Julien! 👍 |
julien4215
force-pushed
the
broadcast-feature
branch
from
c745d92
to
1c111f1
Compare
|
Looks really cool! In the analysis screen (final part of the video), would it make sense to display the players' names above/below the board? The mobile site also does this.
|
Yes, I also think it would be better with the players' names and clock/result. |
|
Yeah, awesome work @julien4215 . I think we can release like that indeed, even without the eval bar. I'll deal with it asap. Right now I'm focusing on releasing the new editor and opening explorer so I think it'll be part of the next release. |
…ound screen to a tab and create overview tab
julien4215
force-pushed
the
broadcast-feature
branch
from
65c5deb
to
1913546
Compare
|
I'm focusing right now on the next release but I'll review asap. Can you please fix the test and solve the conflict in the meantime? Thanks! |
|
I'll fix the tests and the conflict once the PR is fully ready. I have to fix some problems with the clock and the analysis screen. |
In that case I prefer that the PR is a draft. Easier for me to choose PR to review. Thanks! |
…ly the orange box around last move when the game is still ongoing
| final games = | ||
| await ref.watch(broadcastRoundProvider(broadcastRoundId).future); | ||
|
|
||
| _timer = Timer.periodic( |
There was a problem hiding this comment.
I don't understand why you chose to implement the clocks like that, instead of relying on the CountdownClock widget.
Here, I guess it works, but:
- it is not precise (second precision)
- it is not efficient: you rebuild the whole screen and all the games each second; if you have a local state instead, only the clock widget is rebuilt
There was a problem hiding this comment.
We don't need more than second precision for broadcast and I need to share the clock state between two screens (broadcast_screen.dart and broadcast_analysis_screen.dart) so I don't know how you would do it with local state.
There was a problem hiding this comment.
It is not obligatory to share the state. Even more, this state shouldn't be part of the model (of riverpod), but it is best to keep it local. It know it is debatable, but to me it falls in the ephemeral state category, especially since the clocks are "read only" and don't depend on a user action, just on server update events.
You need to distinguish the timer state (when the clock is running down) from the clock state (the real clock time which is held by the server).
There was a problem hiding this comment.
Let's say the user is on the broadcast screen and tap on a board, it brings the user to the broadcast analysis screen and then he goes back to the broadcast screen. All I have is the time a player had when the server sent the clock because when the user moved to the broadcast analysis screen I lost the timer state. So I would need to make another request to the server just to get the time of the clock.
There was a problem hiding this comment.
I still think it is preferable to make this extra request versus having a shared state between 2 screens and a potentially costly build of a screen that can contain a list with many board widgets (think of cheap phones).
There was a problem hiding this comment.
Each second the widget is rebuilded but only the clocks part get repainted so I thought it was fine but by looking at the documentation I see that's still a bad idea to rebuild large part of the tree.
Making an extra request each time add a load on the server that could be avoided so I guess none of these solutions works great.
Maybe I could create a different provider that stores the elapsed time since the request has been sent and that is only watched by the clocks widgets ?
There was a problem hiding this comment.
Testing right now the web version, each time we are changing the board focus, this xhr request is made:
https://lichess.org/study/FOEszqAH/KBwrgngN
The JSON response contains all the data you need to update the clock as far as I can tell. So if the browser does it, we can do also this extra request. This solution is by far the one which adds the less complexity and I really think we should keep things simple.
There was a problem hiding this comment.
The extra request I would need to make is https://lichess.org/api/broadcast/{broadcastTournamentSlug}/{broadcastRoundSlug}/{broadcastRoundId} which can be quite heavy if there are a lot of boards.
There was a problem hiding this comment.
{
"study": {
"id": "hHsVn1AO",
"name": "Round 5",
"members": {},
"position": {
"chapterId": "QIJJTYSZ",
"path": ""
},
"ownerId": "linuxbrickie",
"settings": {
"explorer": "everyone",
"description": false,
"computer": "everyone",
"chat": "everyone",
"sticky": false,
"shareable": "everyone",
"cloneable": "everyone"
},
"visibility": "public",
"createdAt": 1726870475851,
"secondsSinceUpdate": 13,
"from": {
"relay": null
},
"likes": 1,
"liked": false,
"features": {
"cloneable": true,
"shareable": true,
"chat": true
},
"topics": [
"Broadcast"
],
"chapters": [
{
"id": "QIJJTYSZ",
"name": "Zhuang, Kyle - Khanna, Nalin",
"fen": "2r5/pp2bk2/4pp2/3p3p/1n1P1B1P/1P6/P3RPP1/4N1K1 w - - 4 27",
"players": [
{
"name": "Zhuang, Kyle",
"rating": 1987,
"fideId": 30957486,
"fed": "USA",
"clock": 235800
},
{
"name": "Khanna, Nalin",
"rating": 2113,
"fideId": 30919550,
"fed": "USA",
"clock": 179900
}
],
"lastMove": "c6c8",
"thinkTime": 330,
"status": "*"
},
{
"id": "tkYyyGnL",
"name": "Badiee, Barzin - Diao, Matthew Guo",
"fen": "r1r3k1/1p3ppp/3q1n2/nP1p2N1/N2Pp1bP/PQ2P3/5PP1/R3KB1R w KQ - 1 17",
"players": [
{
"name": "Badiee, Barzin",
"rating": 1860,
"fideId": 30962374,
"fed": "USA",
"clock": 213300
},
{
"name": "Diao, Matthew Guo",
"rating": 2097,
"fideId": 30970547,
"fed": "USA",
"clock": 118400
}
],
"lastMove": "c6a5",
"thinkTime": 68,
"status": "*"
},
{
"id": "OdX0sDuk",
"name": "ADU, Oladapo - Yang, Daniel",
"fen": "2r3k1/3q1pb1/p5pp/1p1np3/1P1n4/P2PN1PP/1B3PB1/2Q1R1K1 w - - 1 28",
"players": [
{
"name": "ADU, Oladapo",
"title": "IM",
"rating": 2024,
"fideId": 8500258,
"fed": "NGR",
"clock": 298000
},
{
"name": "Yang, Daniel",
"rating": 2091,
"fideId": 30976340,
"fed": "USA",
"clock": 95800
}
],
"lastMove": "e8c8",
"thinkTime": 55,
"status": "*"
},
{
"id": "i0umHUIG",
"name": "Samuelson, Andrew - Xiong, Lang",
"fen": "1Q6/K2k2N1/8/8/8/8/8/8 b - - 0 78",
"players": [
{
"name": "Samuelson, Andrew",
"title": "FM",
"rating": 2051,
"fideId": 2020580,
"fed": "USA",
"clock": 528800
},
{
"name": "Xiong, Lang",
"rating": 2000,
"fideId": 30957435,
"fed": "USA",
"clock": 341600
}
],
"lastMove": "b7b8q",
"status": "1-0"
},
{
"id": "QG0I72q3",
"name": "Ismayilova, Khanim - Baron, Tal",
"fen": "1n1r2k1/p2b1pbp/3Pp2p/1B2P3/3p4/5N2/q2N1PPP/1R2Q1K1 w - - 0 21",
"players": [
{
"name": "Ismayilova, Khanim",
"title": "WCM",
"rating": 1960,
"fideId": 13429019,
"fed": "AZE",
"clock": 149000
},
{
"name": "Baron, Tal",
"title": "GM",
"rating": 2459,
"fideId": 2809958,
"fed": "ISR",
"clock": 245500
}
],
"lastMove": "c5d4",
"thinkTime": 468,
"status": "*"
},
{
"id": "FGsWkzuW",
"name": "Swaminathan, Pranav - Hebbar, Eshaan",
"fen": "2r1r3/pb3pk1/3Q1bp1/1P6/1Pp5/5N1P/P4PP1/2R3K1 w - - 0 24",
"players": [
{
"name": "Swaminathan, Pranav",
"rating": 2056,
"fideId": 30970482,
"fed": "USA",
"clock": 221500
},
{
"name": "Hebbar, Eshaan",
"title": "CM",
"rating": 2208,
"fideId": 30974690,
"fed": "USA",
"clock": 235800
}
],
"lastMove": "c5c4",
"thinkTime": 914,
"status": "*"
},
{
"id": "qTNxh3Gg",
"name": "Li, Sophie - Feng, Andrew",
"fen": "1r4k1/6p1/4p2p/pq2Pp2/2pBnP2/1pP3P1/PPQ4P/3R2K1 w - - 0 35",
"players": [
{
"name": "Li, Sophie",
"title": "WCM",
"rating": 1865,
"fideId": 30981107,
"fed": "USA",
"clock": 359300
},
{
"name": "Feng, Andrew",
"rating": 2091,
"fideId": 30943256,
"fed": "USA",
"clock": 76100
}
],
"lastMove": "b4b3",
"thinkTime": 55,
"status": "*"
},
{
"id": "GpSrbymH",
"name": "Brinkmann, Nicholas - Moorhouse, Will",
"fen": "1r6/4k2p/1pppb1p1/5p2/2P5/1B2P2P/P4KP1/3R4 b - - 2 29",
"players": [
{
"name": "Brinkmann, Nicholas",
"rating": 1695,
"fideId": 39905314,
"fed": "USA",
"clock": 159900
},
{
"name": "Moorhouse, Will",
"rating": 2091,
"fideId": 30989531,
"fed": "USA",
"clock": 238800
}
],
"lastMove": "f1f2",
"thinkTime": 13,
"status": "*"
}
],
"chapter": {
"id": "tkYyyGnL",
"ownerId": "linuxbrickie",
"setup": {
"variant": {
"key": "standard",
"name": "Standard"
},
"orientation": "white"
},
"tags": [
[
"White",
"Badiee, Barzin"
],
[
"WhiteElo",
"1860"
],
[
"WhiteFideId",
"30962374"
],
[
"Black",
"Diao, Matthew Guo"
],
[
"BlackElo",
"2097"
],
[
"BlackFideId",
"30970547"
],
[
"Result",
"*"
],
[
"Round",
"5.2"
]
],
"features": {
"computer": true,
"explorer": true
},
"relayPath": "-=aP$5WG)8G?8IUM.>VF=FMF2B\\M%@`N@N^N/7]A&;_b+3SK,<`];4KC<DC;5;MC"
},
"federations": {
"USA": "United States of America",
"NGR": "Nigeria",
"ISR": "Israel",
"AZE": "Azerbaijan"
},
"chat": {
"lines": [
{
"u": "dragonsfire66",
"t": "good luck to all",
"p": true,
"f": "nature.crocodile"
},
{
"u": "Archovie16",
"t": "board 4 glitch?"
},
{
"u": "learnedMachine",
"t": "yep should update in a bit"
}
],
"writeable": true
}
},
"analysis": {
"game": {
"id": "synthetic",
"variant": {
"key": "standard",
"name": "Standard",
"short": "Std"
},
"opening": null,
"fen": "rnbqkbnr/pppppppp/8/8/8/8/PPPPPPPP/RNBQKBNR w KQkq - 0 1",
"turns": 0,
"player": "white",
"status": {
"id": 10,
"name": "created"
},
"initialFen": "rnbqkbnr/pppppppp/8/8/8/8/PPPPPPPP/RNBQKBNR w KQkq - 0 1"
},
"player": {
"id": null,
"color": "white"
},
"opponent": {
"color": "black",
"ai": null
},
"orientation": "white",
"pref": {
"animationDuration": 144,
"coords": 2,
"moveEvent": 2,
"showCaptured": true,
"keyboardMove": false,
"rookCastle": true,
"highlight": true,
"destination": true
},
"userAnalysis": true,
"treeParts": [
{
"ply": 0,
"fen": "rnbqkbnr/pppppppp/8/8/8/8/PPPPPPPP/RNBQKBNR w KQkq - 0 1",
"dests": "ksA owE bqs gvx jrz nvD muC ltB iqy pxF"
},
{
"ply": 1,
"fen": "rnbqkbnr/pppppppp/8/8/2P5/8/PP1PPPPP/RNBQKBNR b KQkq - 0 1",
"id": "-=",
"uci": "c2c4",
"san": "c4",
"clock": 545700
},
{
"ply": 2,
"fen": "rnbqkb1r/pppppppp/5n2/8/2P5/8/PP1PPPPP/RNBQKBNR w KQkq - 1 2",
"id": "aP",
"uci": "g8f6",
"san": "Nf6",
"clock": 500500
},
{
"ply": 3,
"fen": "rnbqkb1r/pppppppp/5n2/8/2P5/2N5/PP1PPPPP/R1BQKBNR b KQkq - 2 2",
"id": "$5",
"uci": "b1c3",
"san": "Nc3",
"clock": 547900
},
{
"ply": 4,
"fen": "rnbqkb1r/pppp1ppp/5n2/4p3/2P5/2N5/PP1PPPPP/R1BQKBNR w KQkq - 0 3",
"id": "WG",
"uci": "e7e5",
"san": "e5",
"clock": 502200
},
{
"ply": 5,
"fen": "rnbqkb1r/pppp1ppp/5n2/4p3/2P5/2N2N2/PP1PPPPP/R1BQKB1R b KQkq - 1 3",
"id": ")8",
"uci": "g1f3",
"san": "Nf3",
"clock": 550000
},
{
"ply": 6,
"fen": "rnbqkb1r/pppp1ppp/5n2/8/2P1p3/2N2N2/PP1PPPPP/R1BQKB1R w KQkq - 0 4",
"id": "G?",
"uci": "e5e4",
"san": "e4",
"clock": 504800
},
{
"ply": 7,
"fen": "rnbqkb1r/pppp1ppp/5n2/6N1/2P1p3/2N5/PP1PPPPP/R1BQKB1R b KQkq - 1 4",
"id": "8I",
"uci": "f3g5",
"san": "Ng5",
"clock": 552100
},
{
"ply": 8,
"fen": "rnbqkb1r/pp1p1ppp/2p2n2/6N1/2P1p3/2N5/PP1PPPPP/R1BQKB1R w KQkq - 0 5",
"id": "UM",
"uci": "c7c6",
"san": "c6",
"clock": 507000
},
{
"ply": 9,
"fen": "rnbqkb1r/pp1p1ppp/2p2n2/6N1/2PPp3/2N5/PP2PPPP/R1BQKB1R b KQkq d3 0 5",
"id": ".>",
"uci": "d2d4",
"san": "d4",
"clock": 524400
},
{
"ply": 10,
"fen": "rnbqkb1r/pp3ppp/2p2n2/3p2N1/2PPp3/2N5/PP2PPPP/R1BQKB1R w KQkq - 0 6",
"id": "VF",
"uci": "d7d5",
"san": "d5",
"clock": 428000
},
{
"ply": 11,
"fen": "rnbqkb1r/pp3ppp/2p2n2/3P2N1/3Pp3/2N5/PP2PPPP/R1BQKB1R b KQkq - 0 6",
"id": "=F",
"uci": "c4d5",
"san": "cxd5",
"clock": 496500
},
{
"ply": 12,
"fen": "rnbqkb1r/pp3ppp/5n2/3p2N1/3Pp3/2N5/PP2PPPP/R1BQKB1R w KQkq - 0 7",
"id": "MF",
"uci": "c6d5",
"san": "cxd5",
"clock": 429600
},
{
"ply": 13,
"fen": "rnbqkb1r/pp3ppp/5n2/3p2N1/3Pp2P/2N5/PP2PPP1/R1BQKB1R b KQkq - 0 7",
"id": "2B",
"uci": "h2h4",
"san": "h4",
"clock": 497900
},
{
"ply": 14,
"fen": "r1bqkb1r/pp3ppp/2n2n2/3p2N1/3Pp2P/2N5/PP2PPP1/R1BQKB1R w KQkq - 1 8",
"id": "\\M",
"uci": "b8c6",
"san": "Nc6",
"clock": 392500
},
{
"ply": 15,
"fen": "r1bqkb1r/pp3ppp/2n2n2/3p2N1/3PpB1P/2N5/PP2PPP1/R2QKB1R b KQkq - 2 8",
"id": "%@",
"uci": "c1f4",
"san": "Bf4",
"clock": 459000
},
{
"ply": 16,
"fen": "r1bqk2r/pp3ppp/2nb1n2/3p2N1/3PpB1P/2N5/PP2PPP1/R2QKB1R w KQkq - 3 9",
"id": "`N",
"uci": "f8d6",
"san": "Bd6",
"clock": 280600
},
{
"ply": 17,
"fen": "r1bqk2r/pp3ppp/2nB1n2/3p2N1/3Pp2P/2N5/PP2PPP1/R2QKB1R b KQkq - 0 9",
"id": "@N",
"uci": "f4d6",
"san": "Bxd6",
"clock": 445700
},
{
"ply": 18,
"fen": "r1b1k2r/pp3ppp/2nq1n2/3p2N1/3Pp2P/2N5/PP2PPP1/R2QKB1R w KQkq - 0 10",
"id": "^N",
"uci": "d8d6",
"san": "Qxd6",
"clock": 273500
},
{
"ply": 19,
"fen": "r1b1k2r/pp3ppp/2nq1n2/3p2N1/3Pp2P/2N1P3/PP3PP1/R2QKB1R b KQkq - 0 10",
"id": "/7",
"uci": "e2e3",
"san": "e3",
"clock": 444200
},
{
"ply": 20,
"fen": "r3k2r/pp3ppp/2nq1n2/3p2N1/3Pp1bP/2N1P3/PP3PP1/R2QKB1R w KQkq - 1 11",
"id": "]A",
"uci": "c8g4",
"san": "Bg4",
"clock": 245200
},
{
"ply": 21,
"fen": "r3k2r/pp3ppp/2nq1n2/3p2N1/Q2Pp1bP/2N1P3/PP3PP1/R3KB1R b KQkq - 2 11",
"id": "&;",
"uci": "d1a4",
"san": "Qa4",
"clock": 406700
},
{
"ply": 22,
"fen": "r4rk1/pp3ppp/2nq1n2/3p2N1/Q2Pp1bP/2N1P3/PP3PP1/R3KB1R w KQ - 3 12",
"id": "_b",
"uci": "e8h8",
"san": "O-O",
"clock": 237600
},
{
"ply": 23,
"fen": "r4rk1/pp3ppp/2nq1n2/3p2N1/Q2Pp1bP/P1N1P3/1P3PP1/R3KB1R b KQ - 0 12",
"id": "+3",
"uci": "a2a3",
"san": "a3",
"clock": 400400
},
{
"ply": 24,
"fen": "r4rk1/1p3ppp/p1nq1n2/3p2N1/Q2Pp1bP/P1N1P3/1P3PP1/R3KB1R w KQ - 0 13",
"id": "SK",
"uci": "a7a6",
"san": "a6",
"clock": 204400
},
{
"ply": 25,
"fen": "r4rk1/1p3ppp/p1nq1n2/3p2N1/QP1Pp1bP/P1N1P3/5PP1/R3KB1R b KQ - 0 13",
"id": ",<",
"uci": "b2b4",
"san": "b4",
"clock": 346500
},
{
"ply": 26,
"fen": "r1r3k1/1p3ppp/p1nq1n2/3p2N1/QP1Pp1bP/P1N1P3/5PP1/R3KB1R w KQ - 1 14",
"id": "`]",
"uci": "f8c8",
"san": "Rfc8",
"clock": 142500
},
{
"ply": 27,
"fen": "r1r3k1/1p3ppp/p1nq1n2/3p2N1/1P1Pp1bP/PQN1P3/5PP1/R3KB1R b KQ - 2 14",
"id": ";4",
"uci": "a4b3",
"san": "Qb3",
"clock": 321600
},
{
"ply": 28,
"fen": "r1r3k1/1p3ppp/2nq1n2/p2p2N1/1P1Pp1bP/PQN1P3/5PP1/R3KB1R w KQ - 0 15",
"id": "KC",
"uci": "a6a5",
"san": "a5",
"clock": 126000
},
{
"ply": 29,
"fen": "r1r3k1/1p3ppp/2nq1n2/pP1p2N1/3Pp1bP/PQN1P3/5PP1/R3KB1R b KQ - 0 15",
"id": "<D",
"uci": "b4b5",
"san": "b5",
"clock": 227200
},
{
"ply": 30,
"fen": "r1r3k1/1p3ppp/2nq1n2/1P1p2N1/p2Pp1bP/PQN1P3/5PP1/R3KB1R w KQ - 0 16",
"id": "C;",
"uci": "a5a4",
"san": "a4",
"clock": 117100
},
{
"ply": 31,
"fen": "r1r3k1/1p3ppp/2nq1n2/1P1p2N1/N2Pp1bP/PQ2P3/5PP1/R3KB1R b KQ - 0 16",
"id": "5;",
"uci": "c3a4",
"san": "Nxa4",
"clock": 213300
},
{
"ply": 32,
"fen": "r1r3k1/1p3ppp/3q1n2/nP1p2N1/N2Pp1bP/PQ2P3/5PP1/R3KB1R w KQ - 1 17",
"id": "MC",
"uci": "c6a5",
"san": "Na5",
"clock": 118400
}
]
}
}
Here is the JSON returned by the study chapter XHR from the browser (same request as I mentioned above). It looks like the data returned is pretty similar to the one you mentioned: https://lichess.org/api#tag/Broadcasts/operation/broadcastRoundGet
Instead of games you have chapters, but actually inside it is the same data and you have one chapter per board.
You're right to be caution about extra requests and server work. But again, in our case, if the browser does it, we can do it. Also this is a first version, there's room for improvement and optimisation later. Early optimisation... you know the saying.
This reverts commit 82063f6.
|
I just saw your replies to my comments now @julien4215 . I've responded to all of them, sorry for the late reply. |
There was a problem hiding this comment.
Coming back on it @julien4215
Have you found a way to handle the clocks?
I think the most obvious way to share state between 2 screen is to use a separate provider.
So you need a provider family that can instantiate the clock of each game. Something like:
@riverpod
Future<BroadcastGameClockState> gameClock(BroadcastRoundId roundId, BroadcastGameId gameId) {
// get clock data from game controller
// listen to socket to get clock update events
// instantiate the local timer
return clockData;
}Then you can use this provider in both screens.
|
I didn't have time to think about it but by using select I could avoid rebuilding when it is not needed or by splitting the provider as you said would also work. I'll try to work on it next week. |
I don't think |
Is it a good idea to create as many timers as there are games ? I only need one timer to get clocks of each player. |
|
I know what you mean: the elapsed seconds are the same for everyone. But it is not a problem to create one timer per game if it allows us to solve this issue we're having. In the real tournament, there is also one clock per game. We're just doing the same here. I haven't looked deeply at the broadcast server side and API yet, but I suppose you'd receive a socket event for a clock update for one game at a time? It makes sense to have separate timers for each clock for that reason too. Currently in TV and game screens, each clock you see has its own timer as well. |
screen-20240910-235044.2.mp4
(Recording updated to latest commit)